Skip to content

Conversation

@niklasmohrin
Copy link
Contributor

@niklasmohrin niklasmohrin commented Nov 7, 2025

Closes #136

To test out the installation from the branch:

pip install 'git+https://github.com/niklasmohrin/BAPCtools.git@module'
bt --help

Tasks:

  • fix tests
  • group imports
  • set correct platform requirements for wheel (only Linux, because of checktestdata) edit: we just pretend that it is universal
  • check that everything still works
  • add compatibility script in bin/tools.py (optionally, make it warn about deprecation)

Things to double-check in review:

  • Does shell auto completion still work with the previous setup when just upgrading via git pull?

What this PR does not do, but what can be done next:

  1. Publish to PyPI: need to decide version names, set up workflow, etc. Having the pip install from git is already a nice improvement.
  2. Remove list of dependencies in pre-commit config: Want to change something anyways, so easier to to separately. Current config still works in the meantime. See Make bapctools pip-installable #474 (comment).
  3. Remove list of dependencies / docker container in GH actions: Could be nice to use pip installation there too.
  4. Good pinning for reproducibility in CI: Not sure if it is worth the effort, because doing it right means using something like uv or another extra tool.
  5. Update README with instructions to install with pip: I would defer this change until we have figured out whether we want to publish to PyPI.

@mzuenni

This comment was marked as resolved.

@niklasmohrin

This comment was marked as resolved.

@mzuenni

This comment was marked as resolved.

@mpsijm mpsijm mentioned this pull request Nov 8, 2025
@mpsijm
Copy link
Collaborator

mpsijm commented Nov 8, 2025

@niklasmohrin Looks like there have been some changes on main since this PR was created (partly my fault 😇). Most likely, there will be conflicts in the imports. Do you see a checkbox that "allow[s] edits by maintainers"? If so, I can rebase whenever this PR gets out-of-sync, if you like 🙂

@niklasmohrin
Copy link
Contributor Author

I checked the box now, but actually I think I would prefer to just do the rebase myself, it's not too bad and gives me less anxiety than multiple people force pushing a branch :D

I will not work on this this weekend, so feel free to push as many conflicts as you want to main :) I will write a regex for the rebase

@mpsijm
Copy link
Collaborator

mpsijm commented Nov 9, 2025

Haha, all right! I'll delay the import rewrites from #473 (comment) anyway, but good to hear that the rebasing won't be an issue for you 🙂

@niklasmohrin
Copy link
Contributor Author

niklasmohrin commented Nov 17, 2025

I updated the PR description with the current information and my thoughts on what to do next. I think I am happy with the current state of the PR. Let me know what you think about the PR and the proposed next steps.

When merging: Please squash everything :)

@niklasmohrin niklasmohrin marked this pull request as ready for review November 17, 2025 13:39
@mpsijm
Copy link
Collaborator

mpsijm commented Nov 19, 2025

I'm also happy with the current state and the list of things to do after merging this PR, thanks a ton! 😄 ❤️

Because we have NWERC coming up (30 November), we'll wait with merging this until after that contest. Then we'll also have more time to do a thorough review 🙂 I'll keep using your branch during problem development to see if things break (so far, so good)! In the mean time, feel free to investigate some of the follow-ups if you have the time (also fine if you don't have the time) 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider renaming bin to lib

4 participants